Skip to content

feat: add desktop source path support for app items#1480

Merged
deepin-bot[bot] merged 1 commit intolinuxdeepin:masterfrom
mhduiy:privacy
Mar 5, 2026
Merged

feat: add desktop source path support for app items#1480
deepin-bot[bot] merged 1 commit intolinuxdeepin:masterfrom
mhduiy:privacy

Conversation

@mhduiy
Copy link
Contributor

@mhduiy mhduiy commented Mar 5, 2026

Added desktopSourcePath property to AppItem class to store the desktop file source path from D-Bus application info.
This enables tracking the original desktop file location for applications, which is useful for debugging and application management. The change includes adding new role in AppItemModel, getter/setter methods in AppItem, and initialization in AMAppItem constructor. Also updated build dependency to require newer dde-application-manager- api version that provides the DesktopSourcePath field.

feat: 为应用项添加桌面源路径支持

在AppItem类中添加desktopSourcePath属性,用于存储来自D-Bus应用信息的桌面 文件源路径。
这有助于跟踪应用程序的原始桌面文件位置,对于调试和应用管理非常有用。
更改包括在AppItemModel中添加新角色、在AppItem中添加getter/setter方法以及 在AMAppItem构造函数中进行初始化。
同时更新了构建依赖,要求提供DesktopSourcePath字段的更新版dde-
application-manager-api。

PMS: BUG-351621

Summary by Sourcery

Add support for storing and exposing the desktop file source path for application items.

New Features:

  • Expose a desktopSourcePath property on AppItem, backed by a new DesktopSourcePathRole in AppItemModel.
  • Initialize AppItem.desktopSourcePath from the DesktopSourcePath field in D-Bus application info within AMAppItem.

@sourcery-ai
Copy link

sourcery-ai bot commented Mar 5, 2026

Reviewer's guide (collapsed on small PRs)

Reviewer's Guide

Adds support for tracking the desktop file source path for application items by introducing a new desktopSourcePath property on AppItem, wiring it through the D-Bus-backed AMAppItem constructor, exposing it via a new role in AppItemModel for QML/consumers, and updating build dependencies accordingly.

Sequence diagram for propagating DesktopSourcePath into AppItem

sequenceDiagram
participant DbusService
participant AMAppItem
participant AppItem
participant AppItemModel
participant QMLView

DbusService->>AMAppItem: construct with appInfo (includes DesktopSourcePath)
AMAppItem->>AMAppItem: desktopSourcePath = appInfo.value(DesktopSourcePath).toString()
AMAppItem->>AppItem: setDesktopSourcePath(desktopSourcePath)
AppItem->>AppItemModel: setData(desktopSourcePath, DesktopSourcePathRole)
QMLView->>AppItemModel: data(index, DesktopSourcePathRole)
AppItemModel-->>QMLView: desktopSourcePath
Loading

Class diagram for AppItem desktopSourcePath support

classDiagram

class AppItem {
  +QStringMap execs()
  +void setExecs(QStringMap execs)
  +QString desktopSourcePath()
  +void setDesktopSourcePath(QString desktopSourcePath)
}

class AMAppItem {
  +AMAppItem(QDBusObjectPath path, ObjectInterfaceMap source)
}

class AppItemModel {
  +int DesktopSourcePathRole
  +QHash<int,QByteArray> roleNames()
}

AppItemModel "1" o-- "*" AppItem
AMAppItem --|> AppItem
Loading

File-Level Changes

Change Details Files
Expose desktop file source path as a first-class property on AppItem and through AppItemModel roles for use by views/QML.
  • Add desktopSourcePath getter and setter methods on AppItem that read/write via the model role system
  • Extend AppItemModel::Roles enum with DesktopSourcePathRole and register the corresponding role name in roleNames()
applets/dde-apps/appitem.h
applets/dde-apps/appitem.cpp
applets/dde-apps/appitemmodel.h
applets/dde-apps/appitemmodel.cpp
Initialize AppItem.desktopSourcePath from D-Bus app info and bump dde-application-manager-api dependency to a version that provides this field.
  • Read DesktopSourcePath from the D-Bus appInfo map in AMAppItem constructor and pass it into AppItem via setDesktopSourcePath
  • Update Debian packaging control to require a newer dde-application-manager-api that includes DesktopSourcePath
applets/dde-apps/amappitem.cpp
debian/control

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

Copy link

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey - I've found 1 issue, and left some high level feedback:

  • In appitem.cpp, there appears to be an extra closing brace after AppItem::setDesktopSourcePath (the function already closes with }, then there's another }), which will break compilation and should be removed or matched correctly.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- In appitem.cpp, there appears to be an extra closing brace after AppItem::setDesktopSourcePath (the function already closes with `}`, then there's another `}`), which will break compilation and should be removed or matched correctly.

## Individual Comments

### Comment 1
<location path="applets/dde-apps/appitem.cpp" line_range="229-231" />
<code_context>
+    return data(AppItemModel::DesktopSourcePathRole).toString();
+}
+
+void AppItem::setDesktopSourcePath(const QString &desktopSourcePath)
+{
+    return setData(desktopSourcePath, AppItemModel::DesktopSourcePathRole);
 }
+}
</code_context>
<issue_to_address>
**issue (bug_risk):** Avoid returning a value from a `void` function.

This function is declared `void`, but currently does `return setData(...)`, which is invalid C++. To match the signature, remove `return` and just call `setData(desktopSourcePath, AppItemModel::DesktopSourcePathRole);`. In a follow-up, consider updating `setExecs` to use the same pattern for consistency.
</issue_to_address>

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

Added desktopSourcePath property to AppItem class to store the desktop
file source path from D-Bus application info.
This enables tracking the original desktop file location for
applications, which is useful for debugging and application management.
The change includes adding new role in AppItemModel, getter/setter
methods in AppItem, and initialization in AMAppItem constructor.
Also updated build dependency to require newer dde-application-manager-
api version that provides the DesktopSourcePath field.

feat: 为应用项添加桌面源路径支持

在AppItem类中添加desktopSourcePath属性,用于存储来自D-Bus应用信息的桌面
文件源路径。
这有助于跟踪应用程序的原始桌面文件位置,对于调试和应用管理非常有用。
更改包括在AppItemModel中添加新角色、在AppItem中添加getter/setter方法以及
在AMAppItem构造函数中进行初始化。
同时更新了构建依赖,要求提供DesktopSourcePath字段的更新版dde-
application-manager-api。

PMS: BUG-351621
@deepin-ci-robot
Copy link

deepin pr auto review

这份代码diff主要包含两部分修改:版权年份更新和新增desktopSourcePath功能。下面是对代码的详细审查和改进建议:

1. 版权年份更新

-// SPDX-FileCopyrightText: 2024 UnionTech Software Technology Co., Ltd.
+// SPDX-FileCopyrightText: 2024 - 2026 UnionTech Software Technology Co., Ltd.

评价:这是标准的版权年份更新,将版权保护期限延长至2026年,符合规范。

2. 代码质量改进

2.1 移除多余的return语句

// 修改前
void AppItem::setAppId(const QString &appid)
{
    return setData(appid, AppItemModel::DesktopIdRole);
}

// 修改后
void AppItem::setAppId(const QString &appid)
{
    setData(appid, AppItemModel::DesktopIdRole);
}

评价

  • ✅ 正确移除了void函数中多余的return语句
  • ✅ 这个修改在所有setter方法中统一应用,提高了代码一致性
  • ✅ 符合现代C++编码规范

2.2 新增desktopSourcePath功能

在appitem.h中:

QString desktopSourcePath() const;
void setDesktopSourcePath(const QString &desktopSourcePath);

评价

  • ✅ 声明规范,符合现有的getter/setter模式
  • ✅ 命名清晰,符合Qt命名规范

在appitem.cpp中:

QString AppItem::desktopSourcePath() const
{
    return data(AppItemModel::DesktopSourcePathRole).toString();
}

void AppItem::setDesktopSourcePath(const QString &desktopSourcePath)
{
    setData(desktopSourcePath, AppItemModel::DesktopSourcePathRole);
}

评价

  • ✅ 实现简洁,与现有代码风格一致
  • ⚠️ 建议:参数名与成员变量名相同,虽然Qt中这种写法很常见,但为了提高可读性,可以考虑使用m_前缀或不同的命名

在appitemmodel.h中:

enum Roles {
    // ...其他角色
    DesktopSourcePathRole,
};

评价

  • ✅ 正确添加了新的角色枚举
  • ✅ 位置合理,放在了相关角色附近

在appitemmodel.cpp中:

QHash<int, QByteArray> AppItemModel::roleNames() const
{
    return {{...}, {AppItemModel::DesktopSourcePathRole, QByteArrayLiteral("desktopSourcePath")}};
}

评价

  • ✅ 正确注册了新角色
  • ✅ 使用了QByteArrayLiteral提高性能

在amappitem.cpp中:

auto desktopSourcePath = appInfo.value(u8"DesktopSourcePath").toString();
AppItem::setDesktopSourcePath(desktopSourcePath);

评价

  • ✅ 正确从DBus数据中提取并设置值
  • ⚠️ 建议:可以考虑添加空值检查
    auto desktopSourcePath = appInfo.value(u8"DesktopSourcePath").toString();
    if (!desktopSourcePath.isEmpty()) {
        AppItem::setDesktopSourcePath(desktopSourcePath);
    }

3. 依赖更新

- dde-application-manager-api (>= 1.2.23),
+ dde-application-manager-api (>= 1.2.48),

评价

  • ✅ 正确更新了依赖版本
  • ✅ 保持了向后兼容性要求

4. 代码格式问题

在appitem.cpp文件末尾:

    setData(desktopSourcePath, AppItemModel::DesktopSourcePathRole);
}
+}
\ No newline at end of file

评价

  • ⚠️ 建议:文件末尾应该有一个换行符,这是POSIX标准的要求,也是大多数代码规范的要求

5. 综合改进建议

  1. 安全性

    • 建议对desktopSourcePath进行路径验证,确保它是一个有效的桌面文件路径
    • 可以考虑添加路径格式检查:
      void AppItem::setDesktopSourcePath(const QString &desktopSourcePath)
      {
          if (desktopSourcePath.endsWith(".desktop")) {
              setData(desktopSourcePath, AppItemModel::DesktopSourcePathRole);
          }
      }
  2. 性能

    • 当前实现已经比较高效,使用了QByteArrayLiteral等优化
    • 如果频繁访问desktopSourcePath,可以考虑缓存该值
  3. 文档

    • 建议为新增的desktopSourcePath功能添加注释说明其用途
    • 可以在头文件中添加:
      /**
       * @brief 获取应用程序桌面文件的源路径
       * @return 桌面文件的完整路径
       */
      QString desktopSourcePath() const;
  4. 测试

    • 建议添加单元测试,验证:
      • 正常路径设置
      • 空路径处理
      • 无效路径处理
      • 路径格式验证

总体来说,这次代码修改质量良好,主要改进了代码一致性(移除多余return语句)并添加了新功能(desktopSourcePath)。建议的改进主要是增强健壮性和文档完善。

@deepin-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: BLumia, mhduiy

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@mhduiy
Copy link
Contributor Author

mhduiy commented Mar 5, 2026

/forcemerge

@deepin-bot
Copy link

deepin-bot bot commented Mar 5, 2026

This pr force merged! (status: blocked)

@deepin-bot deepin-bot bot merged commit 5268037 into linuxdeepin:master Mar 5, 2026
9 of 12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants